Fix multiple schemas in config #193
Conversation
WalkthroughThe update to the configuration loading mechanism involves refining the handling of file lists, specifically schema filenames. There is a restructured approach to initializing and assigning file lists within the configuration function, which suggests an enhancement or fix in how the application manages its configuration files. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- config/config.go (2 hunks)
Additional comments: 1
config/config.go (1)
- 169-184: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [139-181]
The changes to
LoadConfigto support multiple schema files by usingStringListseem correct and well-implemented. Ensure that all parts of the codebase that interact withcfg.SchemaFilenameare updated to handle theStringListtype correctly.Verification successful
The search for usages of
cfg.SchemaFilenamewithin Go files has returned only the fileconfig/config.go, which is the file that was modified in the pull request. This suggests that there are no other parts of the codebase usingcfg.SchemaFilenamethat would be affected by the change to theStringListtype. Therefore, no further action is required to update other parts of the codebase.
- The changes to
LoadConfiginconfig/config.goare consistent with the PR's objective to support multiple schema files.- The usage of
cfg.SchemaFilenameappears to be confined toconfig/config.go, indicating no immediate downstream effects on other parts of the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of cfg.SchemaFilename to ensure they handle StringList type. rg --type go 'cfg\.SchemaFilename' --files-with-matchesLength of output: 74
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- config/config.go (2 hunks)
Additional comments: 2
config/config.go (2)
139-142: The initialization of
filesas aStringListis correct and follows Go's idiomatic way of declaring and initializing variables.179-180: The assignment of
filestocfg.SchemaFilenameis correct and reflects the intent to support multiple schema files. This change should be verified to ensure that all parts of the application that usecfg.SchemaFilenamecan handle multiple values correctly.Verification successful
Based on the usages of
cfg.SchemaFilenameinconfig/config.go, it is treated as an iterable, with range loops iterating over its elements. This supports the change to assignfiles(aStringList) tocfg.SchemaFilename, indicating that the application is expected to handle multiple schema filenames correctly.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the application correctly handles multiple schema files. # This script should be run in the root of the repository. rg --type go 'cfg.SchemaFilename' --files-with-matchesLength of output: 71
Script:
#!/bin/bash # Check the usage of 'cfg.SchemaFilename' within 'config/config.go' to determine if it's used as a single string or a list. rg --type go 'cfg.SchemaFilename' config/config.goLength of output: 275
| } | ||
| } | ||
|
|
||
| files := StringList{} | ||
| for _, m := range matches { | ||
| if !files.Has(m) { | ||
| files = append(files, m) | ||
| } | ||
| } | ||
|
|
||
| cfg.SchemaFilename = files | ||
| } | ||
|
|
||
| cfg.SchemaFilename = files | ||
|
|
||
| models := make(config.TypeMap) | ||
| if cfg.Models != nil { | ||
| models = cfg.Models |
There was a problem hiding this comment.
Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [140-176]
The logic for handling glob patterns and appending unique file paths to files appears correct. However, ensure that the StringList.Has method is efficient for larger sets of schema files, as it currently has O(n) complexity for each check.
// Consider using a map for efficient lookups if the number of schema files is large.
Summary by CodeRabbit